Skip to content

Conversation

@aravind-segu
Copy link
Contributor

Introduce Langchain MCP Adapters to make the MCP client easier to use.

Example Notebook: https://e2-spog.staging.cloud.databricks.com/editor/notebooks/367820108530247?o=6051921418418893#command/8418062200632198

workspace_client=WorkspaceClient(),
timeout=30.0,
sse_read_timeout=60.0,
handle_tool_error=True, # Return errors as strings instead of raising
Copy link
Collaborator

@smurching smurching Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ is this handle_tool_error param standard/necessary to start with?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are starting with this for bricks. Bricks is going to use it to handle tool errors and then show the tool call confirmations correctly.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we should still include this since it's a base langchain tool concept, but won't the bricks folks encounter it at get_tools time?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to help usability, is there a langchain doc page we can link to to explain? I saw the docstring above that keyword parameters like hanlde_tool_error get passed through to langchain's connection object, but IDK what that is :D. Maybe in DatabricksMCPServer's docstring we can link the relevant LangChain docs for LangChain's connection object?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@aravind-segu aravind-segu Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh didnt know there were two more types, will add that in. I was trying to have the same parameter here as langchain

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the Langgraph toolNode is different. In Langchain we return BaseTool defined here: https://github.com/langchain-ai/langchain/blob/0a6d01e61df86b70d5b9afdb1ba68e7b2943e787/libs/core/langchain_core/tools/base.py#L483

But I cant find documentation for it

Copy link
Collaborator

@bbqiu bbqiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall LGTM!

did we figure out if we are able to propagate the OAuth error in a meaningful way? i remember you mentioned needing to modify something in the langchain code to surface it? (nb cell link)

Copy link
Collaborator

@smurching smurching left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, just some small comments!

@aravind-segu
Copy link
Contributor Author

overall LGTM!

did we figure out if we are able to propagate the OAuth error in a meaningful way? i remember you mentioned needing to modify something in the langchain code to surface it? (nb cell link)

Yep merged the PR here, will be in the next release: https://github.com/databricks-eng/universe/pull/1446936

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants